Skip to content

fix(tests): isolate fixtures from codemod rewrites and bump Windows debug stack#613

Merged
jamesadevine merged 1 commit into
mainfrom
fix/codemod-race-and-windows-stack
May 18, 2026
Merged

fix(tests): isolate fixtures from codemod rewrites and bump Windows debug stack#613
jamesadevine merged 1 commit into
mainfrom
fix/codemod-race-and-windows-stack

Conversation

@jamesadevine
Copy link
Copy Markdown
Collaborator

Summary

Two surgical fixes for the failing test_1es_compiled_output_no_unreplaced_markers CI test, plus a Windows-only build-config fix discovered while reproducing it.

1. Fix codemod race in compile-fixture tests (tests/compiler_tests.rs)

CI error:

Error: source file /home/runner/work/ado-aw/ado-aw/tests/fixtures/1es-test-agent.md
changed during compilation; refusing to apply codemods. Re-run `ado-aw compile`.

Root cause. The pool_object_form codemod (introduced in 0.30.0) fires on target: 1es fixtures lacking pool:, injecting pool: { name: AZS-1ES-L-MMS-ubuntu-22.04 } and rewriting the source on disk. cargo test runs in parallel, so multiple tests targeting the same fixture all snapshot the original SHA and race to write back — whichever loses trips the lost-update guard in src/compile/mod.rs:305-310.

Fix. Copy each fixture into a per-test temp dir before invoking the compiler. Codemods may still fire (harmless on the temp copy) but the canonical fixture under tests/fixtures/ is never mutated and parallel tests cannot race.

Two edits:

  • test_1es_compiled_output_no_unreplaced_markers (the failing test): now copies 1es-test-agent.md into the temp dir.
  • compile_fixture_with_flags helper (used by ~12 other tests): same pattern.

tests/bash_lint_tests.rs was already doing this correctly. Other inline compile invocations in compiler_tests.rs don't target the 1ES fixtures that trigger the codemod.

2. Fix Windows debug-build stack overflow (.cargo/config.toml)

When reproducing the CI failure locally on Windows, the compile crashed with thread 'main' has overflowed its stack before the codemod logic ever ran — a separate, pre-existing issue.

Root cause. #[tokio::main] polls the top-level future on the main thread. Debug builds produce very large async state machines (no optimisation collapses them), and the deeply-nested async fns in ado-aw compile exceed Windows' default 1 MiB main-thread stack. Linux/macOS default to 8 MiB and were unaffected. Release builds work everywhere because optimisation shrinks the state machines.

Fix. New .cargo/config.toml adds /STACK:8388608 (8 MiB, matching Linux) via a linker flag gated to cfg(all(windows, target_env = "msvc")). Linux and macOS builds are untouched.

Test plan

  • cargo test — full suite passes on Windows (1587 unit + 91 compiler + 0 failures across all test binaries). Before the fix, test_1es_compiled_output_no_unreplaced_markers failed on Windows (stack overflow) and on Linux CI (codemod race).
  • Manually re-ran ado-aw compile tests/fixtures/1es-test-agent.md with the freshly-rebuilt debug binary on Windows — no stack overflow, codemod fires on the temp copy as expected.
  • Verified the canonical fixture under tests/fixtures/ is no longer mutated by test runs (git status clean after cargo test).

…ebug stack

Copy fixtures into a per-test temp dir before invoking ado-aw compile so
the pool_object_form codemod cannot race-rewrite tests/fixtures/*.md
under parallel cargo test, which was tripping the lost-update guard
with 'source file ... changed during compilation' on CI.

Also add .cargo/config.toml setting /STACK:8388608 on Windows MSVC to
match the Linux/macOS 8 MiB default. Debug builds of ado-aw compile
overflow the 1 MiB Windows main-thread stack because #[tokio::main]
polls the top-level future on main and the deeply-nested async fns
produce very large unoptimised state machines.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jamesadevine jamesadevine merged commit a628d09 into main May 18, 2026
13 of 16 checks passed
@jamesadevine jamesadevine deleted the fix/codemod-race-and-windows-stack branch May 18, 2026 09:56
@github-actions
Copy link
Copy Markdown
Contributor

🔍 Rust PR Review

Summary: Looks good — two targeted, correct fixes with no issues found.

Findings

✅ What Looks Good

  • .cargo/config.toml — The /STACK:8388608 flag is correctly gated to cfg(all(windows, target_env = "msvc")), so Linux/macOS builds are entirely unaffected. The value (8 MiB) is the right choice: it matches the Linux default and avoids an arbitrary "large" number. The comment clearly explains the root cause (#[tokio::main] polling on main, large debug async state machines) which is accurate.

  • test_1es_compiled_output_no_unreplaced_markers — Fixture copy is placed before the compiler invocation and uses a matching filename in the temp dir. The fixture_srcfixture_path rename keeps the intent clear. Using expect() here is appropriate for test code.

  • compile_fixture_with_flags — The pattern matches bash_lint_tests.rs, so all test helpers are now consistent. The unwrap_or_else(|e| panic!(...)) form gives a descriptive error message if the copy ever fails, which is better than a bare expect. The explicit -o flag ensures the compiler writes output to the temp dir, not back into tests/fixtures/.

  • Race fix is complete: the only two call sites that compile the 1ES fixture (the direct test and the shared helper) are both covered. Other tests don't target 1ES fixtures, so no other codemods are silently racing.

Generated by Rust PR Reviewer for issue #613 · ● 231K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant